Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: Add team_themes table #2602

Merged
merged 5 commits into from
Jan 2, 2024
Merged

feat: Add team_themes table #2602

merged 5 commits into from
Jan 2, 2024

Conversation

DafyddLlyr
Copy link
Contributor

@DafyddLlyr DafyddLlyr commented Dec 22, 2023 •
edited
Loading

What does this PR do?

  • Moves team.theme JSONB to a new team_themes table
  • Migrates data from column to table
  • Updates frontend queries and types

Relies on theopensystemslab/planx-core#242

@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 22, 2023 •
edited
Loading

🤖 Hasura Change Summary compared a subset of table metadata including permissions:

Tracked Tables (1)

  • public.team_themes permissions:

    insert select update delete
    api ➕
    platformAdmin ➕ ➕
    public ➕
    teamEditor ➕ ➕
    32 added column permissions
    insert select update
    api ➕ favicon
    ➕ id
    ➕ logo
    ➕ primary_colour
    ➕ secondary_colour
    ➕ team_id
    platformAdmin ➕ favicon
    ➕ logo
    ➕ primary_colour
    ➕ secondary_colour
    public
    teamEditor ➕ favicon
    ➕ logo
    ➕ primary_colour
    ➕ secondary_colour

Updated Tables (1)

  • public.teams permissions:

    insert select update delete
    api ➕/➖
    platformAdmin ➕/➖ ➕/➖ ➕/➖
    public ➕/➖
    teamEditor ➕/➖
    6 removed column permissions
    insert select update
    api ➖ theme
    platformAdmin ➖ theme ➖ theme
    public
    teamEditor

@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 22, 2023 •
edited
Loading

Removed vultr server and associated DNS entries

@DafyddLlyr DafyddLlyr mentioned this pull request Dec 22, 2023
Merged
@DafyddLlyr DafyddLlyr force-pushed the dp/team-themes-table branch 2 times, most recently from 8847978 to 6bfa867 Compare December 22, 2023 17:52
DafyddLlyr
DafyddLlyr commented Dec 22, 2023
View reviewed changes
hasura.planx.uk/migrations/1703240593405_squashed/up.sql
FROM
teams;

ALTER TABLE "public"."teams" DROP COLUMN "theme" CASCADE;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Migrating and then dropping the column will work on staging and prod, but on the initial migration on Pizza and locally it won't do anything as the data sync happens after Hasura migrations run.

I tested this locally without dropping the column. If preferred, I can split this into two steps / two PRs.

DafyddLlyr
DafyddLlyr commented Dec 22, 2023
View reviewed changes
scripts/seed-database/write/teams.sql
@@ -3,6 +3,7 @@ CREATE TEMPORARY TABLE sync_teams (
id integer,
name text,
slug text,
-- TODO: Drop this and fetch from team_themes
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO intentionally left here, we'll need to update sync scripts only once this new table is in prod.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @DafyddLlyr

I think this might be related but I was having issues building main since coming back from the holidays.

I was seeing:

psql:write/teams.sql:18: ERROR:  invalid input syntax for type json
DETAIL:  Expected end of input, but found "-11".

Which was resolved by commenting out theme jsonb,

DafyddLlyr
DafyddLlyr commented Dec 22, 2023
View reviewed changes
hasura.planx.uk/migrations/1703240593405_squashed/up.sql
"id" serial NOT NULL,
"team_id" integer NOT NULL,
"primary_colour" text NOT NULL DEFAULT '#0010A4',
"secondary_colour" text,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll check with @ianjon3s in the new year how we should handle defaults for this column.

@DafyddLlyr DafyddLlyr force-pushed the dp/team-themes-table branch from 6bfa867 to fc3aa02 Compare January 2, 2024 08:37
@DafyddLlyr DafyddLlyr merged commit 8db9e42 into main Jan 2, 2024
12 checks passed
@DafyddLlyr DafyddLlyr deleted the dp/team-themes-table branch January 2, 2024 10:08
DafyddLlyr added a commit to theopensystemslab/planx-core that referenced this pull request Jan 2, 2024
@DafyddLlyr
chore: Update team.create() to account for new team_themes table …
a1ca966 
…relationship (#242)

See theopensystemslab/planx-new#2602
This was referenced Jan 2, 2024
Merged
Merged
DafyddLlyr added a commit to theopensystemslab/planx-core that referenced this pull request Jan 3, 2024
@DafyddLlyr
chore: Update TeamTheme type (#254)
af52a96 
This updates `planx-core` types to match new table added in
theopensystemslab/planx-new#2602

Implemented in theopensystemslab/planx-new#2628
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mike-Heneghan Mike-Heneghan Mike-Heneghan left review comments

@jessicamcinchak jessicamcinchak jessicamcinchak approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@DafyddLlyr @jessicamcinchak @Mike-Heneghan